CAMEL-24202: camel-tui accessible from a web browser via --web by ammachado · Pull Request #25317 · apache/camel

ammachado · 2026-08-04T00:45:01Z

Description

Adds --web to camel tui monitor, serving the live TamboUI dashboard over a loopback WebSocket so it can be used from a browser on the same host.

The implementation uses TamboUI's Aesh backend and Aesh terminal bridge. Each browser connection has an independent CamelMonitor / TuiRunner session. It is opt-in and binds only to 127.0.0.1.

This PR also:

Serves vendored xterm.js assets through a Camel-branded page.
Shows the local web :port link in the terminal TUI and prevents a browser session from quitting the local TUI.
Explicitly creates the JLine backend for local terminal sessions.
Rejects cross-origin WebSocket upgrades while allowing the loopback localhost and 127.0.0.1 browser page origins.
Owns and cleanly terminates Netty event loops, and preserves the friendly occupied-port diagnostic.
Clarifies that browser access is limited to the same host; remote access requires an explicit tunnel or equivalent operator setup.

Validation

mvn test -Dtest=TuiWebServerTest -DfailIfNoTests=false
mvn test in dsl/camel-jbang/camel-jbang-plugin-tui (804 tests)

Codex on behalf of ammachado

github-actions · 2026-08-04T00:45:47Z

🌟 Thank you for your contribution to the Apache Camel project! 🌟
🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

First-time contributors require MANUAL approval for the GitHub Actions to run
You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot although they are normally detected and executed by CI.
You can label PRs using skip-tests and test-dependents to fine-tune the checks executed by this PR.
Build and test logs are available in the summary page. Only Apache Camel committers have access to the summary.

⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

github-actions · 2026-08-04T01:04:10Z

🧪 CI tested the following changed modules:

docs
dsl/camel-jbang/camel-jbang-plugin-tui
parent

🔬 Scalpel shadow comparison — Scalpel: 3 tested, 1 compile-only — current: 2 all tested

Maveniverse Scalpel detected 4 affected modules (current approach: 2).

⚠️ Modules only in Scalpel (2)

camel-launcher
docs

Changed properties: aesh-terminal-version

Skip-tests mode would test 3 modules (2 direct + 1 downstream), skip tests for 1 (generated code, meta-modules)

Modules Scalpel would test (3)

camel-jbang-plugin-tui
camel-launcher-container
docs

Modules with tests skipped (1)

camel-launcher

ℹ️ Shadow mode — Scalpel observes but does not affect test execution. Learn more

All tested modules (4 modules)

Camel :: Docs
Camel :: JBang :: Plugin :: TUI
Camel :: Launcher
Camel :: Launcher :: Container

⚙️ View full build and test results

Copilot

Pull request overview

Adds an opt-in --web mode to the Camel JBang TUI so the live monitoring UI can be driven from a web browser via a local (127.0.0.1) WebSocket terminal bridge, alongside the existing local terminal and --mcp server posture.

Changes:

Introduces --web / --web-port options and a TuiWebServer that serves a browser terminal page and accepts WebSocket terminal sessions.
Ensures browser sessions cannot terminate the shared TUI process (e.g., disables quit hints / quit keys for web sessions).
Vendors xterm.js assets and updates documentation (TUI docs + security model) and adds a server-focused test.

Reviewed changes

Copilot reviewed 12 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
parent/pom.xml	Adds aesh terminal-http version property for the new web terminal dependency.
dsl/camel-jbang/camel-jbang-plugin-tui/pom.xml	Adds `tamboui-aesh-backend` + `org.aesh:terminal-http` dependencies to enable web sessions.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiCommand.java	Wires `--web` / `--web-port` through the CLI command arguments.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiBackendHelper.java	Forces explicit JLine backend for local sessions; adds overload to build a runner from a provided backend.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/OverviewTab.java	Hides the “quit” footer hint for web sessions.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/MonitorContext.java	Adds `webSession` flag to adapt UI behavior based on session type.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/CamelMonitor.java	Implements web-server startup, session behavior differences, and header link/badge for local sessions.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiWebServer.java	New web server using Aesh terminal-http WebSocket bootstrap; creates per-connection TUI sessions.
dsl/camel-jbang/camel-jbang-plugin-tui/src/test/java/org/apache/camel/dsl/jbang/core/commands/tui/TuiWebServerTest.java	New tests validating loopback bind, socket close on stop, WebSocket handshake, and static asset serving.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/resources/tui/web/index.html	New Camel-branded browser page wiring xterm.js to the `/ws` endpoint.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/resources/tui/web/vendor/xterm.css	Vendored xterm.js stylesheet.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/resources/tui/web/vendor/xterm-addon-fit.js	Vendored xterm fit addon bundle.
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/resources/tui/web/vendor/LICENSE-xterm.txt	Adds licensing note for vendored xterm.js assets.
docs/user-manual/modules/ROOT/pages/security-model.adoc	Documents `--mcp`/`--web` as local, opt-in management surfaces (threat model framing).
docs/user-manual/modules/ROOT/pages/camel-jbang-tui.adoc	Documents `--web` usage and adds CLI option docs for `--web`/`--web-port`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

gnodet

Nice feature — the WebSocket bridge to xterm.js is cleanly structured and follows the existing MCP server pattern well. Security posture is solid (loopback-only, origin checking, session isolation). Tests are comprehensive.

A few items worth considering:

License headers on vendored files — The vendored xterm.js, xterm.css, and xterm-addon-fit.js have Apache 2.0 license headers prepended, but these are MIT-licensed third-party files (© the xterm.js authors). The LICENSE-xterm.txt with MIT attribution is excellent, but the ASF headers on the files themselves misrepresent copyright. The standard ASF approach is to exclude vendored files from the RAT plugin (e.g., add **/tui/web/vendor/** exclusions in the root pom.xml RAT config) and keep only the original license notice.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

Claude Code on behalf of @gnodet

ammachado · 2026-08-04T13:02:32Z

Fixed the vendored xterm.js license header issue in 8ee58d6: stripped the ASF headers from xterm.js, xterm.css, and xterm-addon-fit.js and excluded them from both the RAT check and the license-maven-plugin header check in the module's own pom.xml, next to the vendor directory they cover. LICENSE-xterm.txt (already correctly MIT-attributed) is untouched and still gets scanned.

Claude Code on behalf of ammachado

ammachado · 2026-08-04T13:53:06Z

@atiaomar1978-hub, could you review this PR?

davsclaus

Well-structured PR with comprehensive tests, proper security documentation, and all prior review feedback addressed.

Non-blocking suggestions

Direct Netty API usage via transitive dependency (Low) — TuiWebServer.java directly imports ~18 io.netty.* classes, but the pom.xml only declares org.aesh:terminal-http (Netty arrives transitively). Since terminal-http exposes Netty types in its own API (TtyWebSocketFrameHandler, HttpRequestHandler), this is largely unavoidable and acceptable for now, but an explicit Netty dependency in the pom.xml would make the contract clearer and prevent breakage if terminal-http ever adjusts its transitive scope.
Origin null allowed in WebSocket check (Very low) — isAllowedOrigin() returns true when no Origin header is present, which means non-browser tools (curl, etc.) can connect. This is fine given loopback-only binding and matches the documented "no authentication beyond loopback" posture, but worth noting as an intentional choice.

Positive observations

Security model updated (security-model.adoc and operator hardening checklist) — --web framing matches the existing --mcp posture.
7 tests in TuiWebServerTest covering TCP binding, shutdown, origin rejection/acceptance, port conflict, event loop termination, WebSocket handshake, and static asset serving. Uses @Isolated, AvailablePortFinder, AssertJ — all following project conventions.
All gnodet review comments (bounded thread pool, awaitTermination covering sessionExecutor, FQCN import) resolved.
Correctly forces JLine backend for local sessions to prevent ServiceLoader picking AeshBackend.
Browser sessions correctly blocked from quitting the host TUI process.
Vendored xterm.js properly excluded from RAT/license checks with LICENSE included.

This review checks the PR against project rules and conventions. It does not replace specialized tools (CodeRabbit, Sourcery, SonarCloud) or a deep static analysis pass.

This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.

ammachado · 2026-08-04T14:55:01Z

Addressed both non-blocking suggestions in b42b60b:

Netty dependency — added explicit io.netty:netty-common, netty-transport, netty-handler, and netty-codec-http dependencies to pom.xml (no version pinned, they resolve through the existing netty-bom import at the same 4.2.16.Final that terminal-http already pulls transitively — verified with dependency:tree, no version drift).
Origin null handling — added a comment on isAllowedOrigin() explaining it's intentional (non-browser clients don't send Origin; loopback-only bind is the actual security boundary), per your note that this is fine as-is.

Claude Code on behalf of ammachado

davsclaus · 2026-08-04T16:07:23Z

sorry you need to rebase as tui code changed

atiaomar1978-hub

Code review — BugBot + Grok

Reviewed PR #25317 (CAMEL-24202: camel tui --web). Overall the loopback bind, security-model documentation, origin rejection for foreign hosts, and test coverage for static assets are solid.

Summary: 3 medium issues worth addressing before merge (session shutdown race, clickjaming on loopback, unbounded session admission). Several low-severity items on quit UX consistency, test isolation, and client cleanup.

Cursor (BugBot + Grok) on behalf of atiaomar1978-hub

atiaomar1978-hub · 2026-08-04T16:38:00Z

+        channels.close().syncUninterruptibly();
+        bossGroup.shutdownGracefully(0, 5, TimeUnit.SECONDS).syncUninterruptibly();
+        workerGroup.shutdownGracefully(0, 5, TimeUnit.SECONDS).syncUninterruptibly();
+        sessionExecutor.shutdownNow();


[Medium — BugBot] stop() calls sessionExecutor.shutdownNow() immediately after closing Netty channels, without waiting for in-flight accept() tasks to finish. When the local terminal exits while browser tabs are connected, CamelMonitor.call() finally blocks can race with event-loop teardown and be interrupted mid-cleanup.

Suggestion: Close channels first, then shutdown() + awaitTermination on sessionExecutor (with a timeout) before shutting down Netty groups; only then shutdownNow() as a last resort.

atiaomar1978-hub · 2026-08-04T16:38:00Z

+
+        @Override
+        protected void channelRead0(ChannelHandlerContext context, FullHttpRequest request) {
+            if ("/ws".equalsIgnoreCase(request.uri()) && !isAllowedOrigin(request)) {


[Medium — Grok] Origin check uses exact "/ws".equalsIgnoreCase(request.uri()). If the upgrade URI includes a query string (e.g. /ws?token=…), this check is skipped and a foreign Origin may reach the WebSocket handler.

Suggestion: Parse the path only (QueryStringDecoder or strip ?…) before comparing, or use startsWith("/ws") with a boundary check.

atiaomar1978-hub · 2026-08-04T16:38:00Z

+        String origin = request.headers().get(HttpHeaderNames.ORIGIN);
+        // A missing Origin header is allowed on purpose: non-browser clients (curl, custom
+        // terminal clients) don't send one, and the loopback-only bind is the actual boundary here.
+        return origin == null || origin.equals("http://127.0.0.1:" + port) || origin.equals("http://localhost:" + port);


[Info] Missing Origin is intentionally allowed for non-browser clients (curl, tests). This is consistent with the security-model doc (loopback bind is the trust boundary), but worth noting: any local process can open a WebSocket session without origin validation.

atiaomar1978-hub · 2026-08-04T16:38:00Z

+    private final ChannelGroup channels = new DefaultChannelGroup(ImmediateEventExecutor.INSTANCE);
+    private final EventLoopGroup bossGroup = new MultiThreadIoEventLoopGroup(1, NioIoHandler.newFactory());
+    private final EventLoopGroup workerGroup = new MultiThreadIoEventLoopGroup(NioIoHandler.newFactory());
+    private final ExecutorService sessionExecutor = Executors.newFixedThreadPool(


[Medium — Grok] Fixed-size pool but Executors.newFixedThreadPool uses an unbounded task queue. Each WebSocket accept enqueues a full CamelMonitor.call(); a localhost client opening many connections can queue unbounded work and exhaust memory/CPU even though only max(4, 2×cpus) run concurrently.

Suggestion: Consider a bounded queue with a rejection policy, connection limit, or semaphore on admissions.

atiaomar1978-hub · 2026-08-04T16:38:00Z

+    }
+
+    private void accept(Connection connection) {
+        sessionExecutor.submit(() -> {


[Low — Grok] Bare sessionExecutor.submit(...) with no RejectedExecutionException handling. After stop() calls shutdownNow(), a late WebSocket accept on a Netty thread can throw on the event loop.

Suggestion: Catch RejectedExecutionException in accept() and close the connection cleanly.

atiaomar1978-hub · 2026-08-04T16:38:00Z

+        }
+    }
+
+    @Test


[Low — coverage gap] Tests cover 127.0.0.1 origin and foreign origin, but not http://localhost:<port> (also allowed by isAllowedOrigin). Worth adding for symmetry.

atiaomar1978-hub · 2026-08-04T16:38:01Z

+          }
+
+          window.addEventListener('resize', handleResize);
+          if (window.ResizeObserver) {


[Low — Grok] On disconnect, window.removeEventListener('resize', …) runs but the ResizeObserver is never disconnected and the Terminal instance is never disposed. Reloading is required to GC; long-lived tabs that disconnect/reconnect would leak listeners.

Suggestion: Store observer reference and call disconnect() in onclose; call term.dispose() if available.

atiaomar1978-hub · 2026-08-04T16:38:01Z

+
+Like the MCP server, the web server is bound to `127.0.0.1` only -- it never listens on
+external interfaces -- and there is no authentication beyond that. Each browser connection
+gets its own independent TUI session (its own process discovery and navigation state), the


[Low — docs] States browser sessions have "the same … keyboard shortcuts" as local terminal, but web sessions hide the q/quit footer hint (OverviewTab) and swallow q/Ctrl+C (CamelMonitor L795–798). Consider noting that quit behaves differently in browser sessions.

atiaomar1978-hub · 2026-08-04T16:38:01Z

+        // Build the JLine backend explicitly rather than leaving backend selection to
+        // TamboUI's ServiceLoader-based auto-discovery: with tamboui-aesh-backend also on the
+        // classpath (for --web), auto-discovery can pick AeshBackend for the local session too,
+        // which drives a native PosixSysTerminal that doesn't shut down cleanly here.


[Info — looks good] Explicit JLine backend avoids ServiceLoader picking AeshBackend for the local session when tamboui-aesh-backend is on the classpath. Good fix for clean shutdown.

atiaomar1978-hub · 2026-08-04T16:38:01Z

  "MBean operation X executes code or sends to endpoint Y when invoked
  from a JMX or Jolokia connection" describes the documented contract,
  not a framework vulnerability.
+* *The Camel TUI's `--mcp` and `--web` servers.* Camel TUI


[Info — looks good] Clear framing of --web alongside --mcp as opt-in loopback management surfaces. Matches Camel's documented trust model.

atiaomar1978-hub · 2026-08-04T16:39:29Z

Full review — BugBot + Grok (CAMEL-24202: `camel tui --web`)

Reviewed #25317 on branch CAMEL-24202.

Cursor (BugBot + Grok) on behalf of atiaomar1978-hub

Overall assessment

This PR adds a useful, opt-in --web mode for the Camel TUI: loopback-only WebSocket terminal via Aesh/TamboUI, vendored xterm.js UI, per-browser-session CamelMonitor instances, and good documentation updates in both the user manual and security model.

Recommendation: Approve after addressing the medium items below (especially session shutdown and clickjaming). The design direction is sound and aligns with how --mcp is already framed in the security model.

What looks good

Loopback bind — TuiWebServer binds explicitly to 127.0.0.1 (not 0.0.0.0).
Security-model documentation — Clear framing of --web alongside --mcp as opt-in management surfaces with no auth beyond loopback.
Origin rejection — Foreign origins get 403 on WebSocket upgrade (tested in TuiWebServerTest).
JLine backend fix — TuiBackendHelper explicitly creates JLine for local sessions, avoiding Aesh auto-discovery when tamboui-aesh-backend is on the classpath.
Signal handler scoping — Signal.handle(INT) is registered only for the local terminal session, not browser sessions.
Browser session isolation — Each WebSocket connection gets its own CamelMonitor / TuiRunner in TuiWebServer.accept().
Static asset tests — Index page, xterm.js/css, fit addon, and logo are covered.
License handling — Vendored xterm assets excluded from RAT with MIT license file.
Port conflict UX — Friendly BindException message with --web-port hint, mirroring MCP.

Issues to address (medium)

1. Session shutdown race — `TuiWebServer.java:121-133`

stop() closes Netty channels then immediately calls sessionExecutor.shutdownNow() without waiting for in-flight accept() tasks. When the local terminal exits while browser tabs are connected, CamelMonitor.call() finally blocks can race with event-loop teardown and be interrupted mid-cleanup.

Suggestion: After closing channels, call sessionExecutor.shutdown() + awaitTermination(timeout) before shutdownNow() as a last resort.

2. Clickjacking on loopback — `TuiWebServer.java:173` + `index.html`

The served page has no X-Frame-Options: DENY or Content-Security-Policy: frame-ancestors 'none'. A remote site can iframe http://127.0.0.1:<port>/; the browser then sends a loopback Origin on WebSocket upgrade (which passes isAllowedOrigin), enabling clickjacking of the full TUI on the loopback trust boundary.

Suggestion: Add frame-denial headers via a small Netty handler or in the static response path.

3. Unbounded session admission — `TuiWebServer.java:76-81`, `142-161`

Executors.newFixedThreadPool uses an unbounded task queue. Each WebSocket accept enqueues a full CamelMonitor.call(). A localhost client opening many connections can queue unbounded work and exhaust memory/CPU even though only max(4, 2×cpus) sessions run concurrently.

Suggestion: Bounded queue + rejection policy, connection limit, or admission semaphore.

4. Origin check URI bypass — `TuiWebServer.java:183-196`

Origin validation uses exact "/ws".equalsIgnoreCase(request.uri()). If the upgrade URI includes a query string (e.g. /ws?token=…), the check is skipped.

Suggestion: Compare path only (QueryStringDecoder or strip query before compare).

5. Inconsistent quit behavior — `CamelMonitor.java:793-798`

Comment says browser session "views the shared monitor" and must not quit the process others use — but TuiWebServer.accept() creates an independent CamelMonitor per connection. Swallowing q/Ctrl+C prevents the browser user from closing their own session, while AI /quit (AiSlashCommandRegistry → requestExit() → tui::quit()) still exits that session.

Suggestion: Allow q/Ctrl+C to call runner.quit() for webBackend != null (only that session), or also block /quit in web sessions — and fix the misleading comment.

6. Test isolation — `TuiWebServerTest.java:98-107`, `133-147`

Successful 101 handshake triggers TtyWebSocketFrameHandler → accept() → full CamelMonitor.call() on a background thread. Tests only assert the status line but may spawn heavyweight monitor sessions with no guaranteed teardown before @AfterEach stop().

Suggestion: Stub accept callback in unit tests, or abort connection immediately after handshake assertion.

Low severity

Location	Finding
`TuiWebServer.java:143`	Bare `submit()` — `RejectedExecutionException` possible after `stop()`; close connection cleanly on reject
`TuiWebServer.java:135-139`	`awaitTermination(timeout)` can block ~2× requested timeout (boss + worker each get full timeout)
`index.html:247-255`	On disconnect, `ResizeObserver` never disconnected and `Terminal` never disposed — listener leak until reload
`TuiWebServerTest.java`	Missing test for `http://localhost:<port>` origin (allowed by `isAllowedOrigin`)
`camel-jbang-tui.adoc:779`	Docs say "same keyboard shortcuts" but web sessions hide quit hint and swallow `q`/Ctrl+C

Informational (by design / looks correct)

Location	Note
`TuiWebServer.java:192-197`	Missing `Origin` allowed for non-browser clients; any local process can open WS — consistent with loopback trust model
`CamelMonitor.java:578`	Process-wide signal handler correctly scoped to local session only
`TuiBackendHelper.java:36`	Explicit JLine backend prevents Aesh auto-discovery shutdown issues
`security-model.adoc:644`	Security framing for `--web` / `--mcp` is accurate and helpful

Test plan suggestions

Manual: camel tui --web, open http://127.0.0.1:8090, verify TUI renders and keyboard works
Manual: Open two browser tabs — confirm independent sessions (separate process discovery state)
Manual: Quit local terminal while browser tab open — verify browser session fails gracefully
Manual: Attempt iframe embed from external origin — verify frame-denial after fix
Automated: Add localhost origin handshake test
Automated: Avoid spawning full CamelMonitor in lightweight handshake tests

Inline comments

Detailed line-level comments are attached in the review thread:
#25317 (review)

Thanks for the contribution — --web is a nice complement to --mcp for local development workflows.

atiaomar1978-hub · 2026-08-04T17:20:45Z

Re-review — follow-up on prior BugBot + Grok comments

Re-checked PR head 265946e1 against the review posted at b42b60bb.

Cursor on behalf of atiaomar1978-hub

Summary

Most of the original medium/low findings are still open. The author did land some improvements (notably in a46d7ed / 265946e1), but TuiWebServer.java, TuiWebServerTest.java, and index.html are unchanged since the prior review commit — none of the line-level issues called out in the earlier thread have been patched yet.

Fixed / improved since original review

Item	Status	Notes
Unbounded `CachedThreadPool`	Fixed	Now `newFixedThreadPool(max(4, 2×cpus))` (`a46d7ed`)
`awaitTermination()` ignores session executor	Fixed	Now waits on `sessionExecutor` with a shared deadline (`a46d7ed`)
Missing `Origin` undocumented	Fixed	Comment added in `isAllowedOrigin()` (`265946e1`)
Explicit Netty dependencies	Fixed	Declared in `pom.xml` instead of transitive-only (`265946e1`)
Vendored xterm license headers	Fixed	ASF headers removed; RAT/license excludes added (`a46d7ed`)
Loopback-only docs wording	Fixed	`camel-jbang-tui.adoc` clarified same-host access (`91cf90e8`)
`BindException` FQCN	Fixed	Proper import in `CamelMonitor` (`a46d7ed`)
Custom Netty pipeline + origin check	Present	From `e23a572` hardening commit

Still open (original review items)

Severity	Location	Issue	Status
Medium	`TuiWebServer.java:121-132`	`stop()` still calls `shutdownNow()` immediately — no graceful drain of in-flight browser sessions before tearing down Netty	Open
Medium	`TuiWebServer.java:173` + `index.html`	No `X-Frame-Options` / CSP `frame-ancestors` — clickjacking via iframe on loopback still possible	Open
Medium	`TuiWebServer.java:76-81`	Fixed pool but unbounded queue — many connections can still queue unbounded `CamelMonitor.call()` tasks	Open
Medium	`TuiWebServer.java:183`	Origin check skipped when URI is `/ws?…` (exact match only)	Open
Medium	`CamelMonitor.java:793-798`	Misleading comment (“shared monitor”); `q`/Ctrl+C swallowed but AI `/quit` still exits session	Open
Medium	`TuiWebServerTest.java:98-147`	Successful handshake still spawns full `CamelMonitor` with weak teardown	Open
Low	`TuiWebServer.java:143`	No `RejectedExecutionException` handling after `stop()`	Open
Low	`TuiWebServer.java:135-139`	`awaitTermination` still gives boss + worker each the full timeout (only remainder goes to executor)	Partial
Low	`index.html:247-255`	`ResizeObserver` / `Terminal` not cleaned up on disconnect	Open
Low	`TuiWebServerTest.java`	No test for `http://localhost:<port>` origin	Open
Low	`camel-jbang-tui.adoc:779`	Docs still say “same keyboard shortcuts”; quit differs in browser	Open

Recommendation

Good progress on thread-pool bounds, termination waiting, docs, and dependency hygiene. Before merge, I'd still want at least:

Graceful session shutdown in stop() (or document that open browser tabs are force-killed).
Frame-denial headers on the HTML response (clickjacking).
Fix or document the /ws?… origin-check bypass.
Align quit behavior + comment in CamelMonitor (either allow browser q for that session only, or block /quit too).

Happy to re-review once these land.

Adds an opt-in --web/--web-port flag that serves the TUI dashboard to a browser over WebSocket, using TamboUI's Aesh backend and Aesh's terminal-http Netty bootstrap. Each browser connection gets its own independently-scanning CamelMonitor session (same as running a second local terminal session), driven through an AeshBackend instead of the JLine backend used for local terminals. Matches the existing TuiMcpServer security posture: loopback-only bind, opt-in, no authentication for v1. Closes the corresponding documentation gap in security-model.adoc for both --mcp and --web. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…ble link, quit gating - Vendor xterm.js/xterm-addon-fit instead of loading from a CDN, and serve a Camel-branded page (logo, disconnect overlay) instead of Aesh's default. - Show the "web :port" badge as an underlined OSC 8 hyperlink in the local terminal session only (hidden once a browser session is attached). - A browser session can no longer quit the shared TUI process via q/Ctrl+C, and the footer hint no longer advertises quit for web sessions. - Build the JLine backend explicitly in TuiBackendHelper so tamboui's ServiceLoader auto-discovery doesn't pick AeshBackend for local sessions now that tamboui-aesh-backend is also on the classpath. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Co-authored-by: Codex <noreply@openai.com>

- Bound the session executor thread pool instead of an unbounded cached pool. - Wait for the session executor to terminate in awaitTermination(), not just the Netty event loops. - Replace FQCN java.net.BindException with an import in CamelMonitor. - Strip the incorrectly-added ASF headers from the vendored MIT-licensed xterm.js/xterm.css/xterm-addon-fit.js assets, and exclude them from both the RAT and license-maven-plugin checks in the module's own pom.xml. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Declares the Netty artifacts TuiWebServer imports directly (netty-common, netty-transport, netty-handler, netty-codec-http) instead of relying on them arriving transitively via org.aesh:terminal-http, and documents why isAllowedOrigin() intentionally allows a missing Origin header. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Fixes 11 issues flagged by an automated review of the web TUI transport layer: - Wait for in-flight sessions to drain (with a timeout) before forcing shutdown, instead of interrupting them immediately in stop(). - Compare the request path (not the raw URI) when deciding whether to enforce the Origin check, closing a bypass via query strings on /ws. - Bound the session executor's queue and reject new sessions once full, instead of queueing unbounded work. - Add X-Frame-Options and Content-Security-Policy response headers to deny framing of the served pages. - Handle RejectedExecutionException when submitting a session so a connection is closed cleanly instead of being silently dropped. - Track a single shared deadline across all three shutdown waits in awaitTermination(), instead of allowing up to 2x the requested timeout. - Dispose the ResizeObserver and Terminal instance on disconnect in the browser client, instead of leaking them until a page reload. - Let tests substitute a lightweight session handler instead of always spawning a full CamelMonitor for WebSocket handshake checks, and add origin coverage for the http://localhost:<port> case. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

ammachado · 2026-08-04T17:52:01Z

Addressed all 11 actionable items from @atiaomar1978-hub's automated review (Cursor BugBot + Grok):

Fixed session shutdown to drain in-flight sessions (with timeout) instead of interrupting immediately.
Fixed origin-check bypass via query strings on /ws (now compares the path, not the raw URI).
Bounded the session executor's queue with rejection instead of unbounded queueing.
Added X-Frame-Options / Content-Security-Policy: frame-ancestors 'none' to deny framing (clickjacking mitigation).
Handled RejectedExecutionException on session submit so connections close cleanly.
Fixed awaitTermination() to track a single shared deadline instead of allowing up to 2x the requested timeout.
Fixed the web-session quit inconsistency (q now quits the session consistently, matching local terminal behavior) and removed the now-dead webSession flag.
Disposed the ResizeObserver and Terminal instance on disconnect in the browser client instead of leaking them until reload.
Let tests substitute a lightweight session handler instead of always spawning a full CamelMonitor for handshake checks, and added origin coverage for http://localhost:<port>.
Confirmed the docs' "same keyboard shortcuts" claim is now accurate given the quit-consistency fix, so no doc change was needed there.

All 861 tests in camel-jbang-plugin-tui pass.

Claude Code on behalf of ammachado

atiaomar1978-hub · 2026-08-04T18:01:29Z

Re-review #3 — automated review findings addressed ✅

Re-checked PR head 51bce14e (CAMEL-24202: address automated review findings on web TUI).

Cursor on behalf of atiaomar1978-hub

Verdict: Ready to approve (with optional nits below)

Commit 51bce14 directly addresses all medium items and most low items from the prior BugBot + Grok review. Nice, focused transport-layer hardening.

Issue tracker — prior review vs current head

Prior finding	Status	Evidence
`stop()` immediate `shutdownNow()` race	Fixed	`shutdown()` + `awaitTermination(5s)` before `shutdownNow()` fallback
Clickjacking (no frame denial)	Fixed	`SecurityHeadersHandler`: `X-Frame-Options: DENY` + `CSP: frame-ancestors 'none'`
Unbounded session queue	Fixed	`ThreadPoolExecutor` + `ArrayBlockingQueue(64)` + `RejectedExecutionException` handling
Origin check `/ws?…` bypass	Fixed	`QueryStringDecoder(request.uri()).path()` before compare
Quit gating / wrong comment	Fixed	Removed `webSession` flag; browser `q`/Ctrl+C now quits this session; comment corrected
Handshake tests spawn full `CamelMonitor`	Fixed	`newServerWithNoOpSession()` + injectable `sessionHandler` constructor
`RejectedExecutionException` after stop	Fixed	Caught in `accept()`, connection closed cleanly
`awaitTermination` ~2× timeout	Fixed	Single shared deadline across boss/worker/session waits
`index.html` ResizeObserver / Terminal leak	Fixed	`disconnect()`, `clearTimeout`, `term.dispose()` on close
Missing `localhost` origin test	Fixed	`acceptsWebSocketUpgradeFromTheLoopbackHostname`
Docs “same shortcuts” vs quit	Fixed	Behavior now matches docs (`q` works in browser sessions)

Optional nits (non-blocking)

Test coverage gap — servesTheCustomIndexPageWithVendoredAssets could assert X-Frame-Options / Content-Security-Policy response headers now that SecurityHeadersHandler exists.
Test coverage gap — no explicit test for foreign origin on /ws?token=… (the path parsing fix is correct in code; a regression test would lock it in).
Test coverage gap — no test exercising queue-full / session rejection (would document the 64-slot admission limit).

These are polish items only; the production code looks solid.

Code quality notes (looks good)

Injectable sessionHandler for tests is clean — no need to mock Netty.
Graceful shutdown ordering (close channels → drain sessions → tear down Netty) is sensible.
Removing the special-case webSession plumbing simplifies CamelMonitor / OverviewTab nicely.

LGTM from the automated review perspective. Thanks for the quick turnaround on 51bce14.

atiaomar1978-hub

Approved ✅

Reviewed head 51bce14e after the automated-review fixes. All prior medium/low findings are addressed:

Graceful session shutdown with timeout before force-stop
Clickjacking mitigated (X-Frame-Options + CSP frame-ancestors 'none')
Bounded session executor queue (64) with clean rejection handling
Origin check uses request path (no /ws?… bypass)
Browser sessions can quit independently; docs and behavior aligned
Lightweight no-op session handler for transport tests
Client-side cleanup on disconnect (ResizeObserver, term.dispose())

Loopback bind, security-model documentation, and origin validation remain sound. LGTM.

Cursor on behalf of atiaomar1978-hub

github-actions Bot added docs dsl core-build-and-dependencies labels Aug 4, 2026

ammachado requested a review from Copilot August 4, 2026 01:14

Copilot started reviewing on behalf of ammachado August 4, 2026 01:14 View session

Copilot AI reviewed Aug 4, 2026

View reviewed changes

Comment thread docs/user-manual/modules/ROOT/pages/camel-jbang-tui.adoc Outdated

oscerd approved these changes Aug 4, 2026

View reviewed changes

ammachado marked this pull request as ready for review August 4, 2026 12:07

gnodet reviewed Aug 4, 2026

View reviewed changes

davsclaus approved these changes Aug 4, 2026

View reviewed changes

atiaomar1978-hub reviewed Aug 4, 2026

View reviewed changes

ammachado force-pushed the CAMEL-24202 branch from b42b60b to 265946e Compare August 4, 2026 17:02

ammachado and others added 6 commits August 4, 2026 13:34

CAMEL-24202: harden web TUI server

c3810ff

Co-authored-by: Codex <noreply@openai.com>

CAMEL-24202: clarify web TUI loopback access

16f3d63

Co-authored-by: Codex <noreply@openai.com>

ammachado force-pushed the CAMEL-24202 branch from 265946e to 276166d Compare August 4, 2026 17:35

atiaomar1978-hub approved these changes Aug 4, 2026

View reviewed changes

davsclaus added the enhancement New feature or request label Aug 4, 2026

davsclaus assigned ammachado Aug 4, 2026

davsclaus added this to the 4.22.0 milestone Aug 4, 2026

davsclaus merged commit 4a9d3f1 into apache:main Aug 4, 2026
6 checks passed

+                      }
+                  }
+                  @Test

Uh oh!

Conversation

ammachado commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Validation

Uh oh!

github-actions Bot commented Aug 4, 2026

Uh oh!

github-actions Bot commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

gnodet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ammachado commented Aug 4, 2026

Uh oh!

ammachado commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

davsclaus left a comment

Choose a reason for hiding this comment

Non-blocking suggestions

Positive observations

Uh oh!

ammachado commented Aug 4, 2026

Uh oh!

davsclaus commented Aug 4, 2026

Uh oh!

atiaomar1978-hub left a comment

Choose a reason for hiding this comment

Code review — BugBot + Grok

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

atiaomar1978-hub commented Aug 4, 2026

Full review — BugBot + Grok (CAMEL-24202: camel tui --web)

Overall assessment

What looks good

Issues to address (medium)

1. Session shutdown race — TuiWebServer.java:121-133

2. Clickjacking on loopback — TuiWebServer.java:173 + index.html

3. Unbounded session admission — TuiWebServer.java:76-81, 142-161

4. Origin check URI bypass — TuiWebServer.java:183-196

5. Inconsistent quit behavior — CamelMonitor.java:793-798

6. Test isolation — TuiWebServerTest.java:98-107, 133-147

Low severity

Informational (by design / looks correct)

Test plan suggestions

Inline comments

Uh oh!

atiaomar1978-hub commented Aug 4, 2026

Re-review — follow-up on prior BugBot + Grok comments

ammachado commented Aug 4, 2026 •

edited

Loading

github-actions Bot commented Aug 4, 2026 •

edited

Loading

ammachado commented Aug 4, 2026 •

edited

Loading

Full review — BugBot + Grok (CAMEL-24202: `camel tui --web`)

1. Session shutdown race — `TuiWebServer.java:121-133`

2. Clickjacking on loopback — `TuiWebServer.java:173` + `index.html`

3. Unbounded session admission — `TuiWebServer.java:76-81`, `142-161`

4. Origin check URI bypass — `TuiWebServer.java:183-196`

5. Inconsistent quit behavior — `CamelMonitor.java:793-798`

6. Test isolation — `TuiWebServerTest.java:98-107`, `133-147`